8365192: post_meth_exit should be in vm state when calling get_jvmti_thread_state #27112
Conversation
|
👋 Welcome back lmesnik! A progress list of the required criteria for merging this PR into |
|
@lmesnik This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 52 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
openjdk
bot
added
serviceability
Webrevs
|
| // At this point we only have the address of a "raw result" and | ||
| // we just call into the interpreter to convert this into a jvalue. | ||
| // The post_method_exit_transition always makes transition to vm and back | ||
| // where GC can happen. So it is needed to preserve result and then restore it |
There was a problem hiding this comment.
| // where GC can happen. So it is needed to preserve result and then restore it | |
| // where GC can happen. So it is needed to preserve result and then restore it |
| } | ||
| BasicType type = current_frame.interpreter_frame_result(&oop_result, &value); | ||
| assert(type == T_VOID || current_frame.interpreter_frame_expression_stack_size() > 0, | ||
| "Stack shouldn't be empty"); |
There was a problem hiding this comment.
I think this assertion needs an adaptation. The expression stack can be empty for native method calls. The result does not necessarily need to be on the expression stack depending on the platform.
Or what else is the reason it is expected to be non-empty?
There was a problem hiding this comment.
Thanks for adding the native check! Looks good to me, now.
Co-authored-by: David Holmes <[email protected]>
This is the second attempt to fix method
post_meth_exitto correctly set state and preserve result.The related fix here:
b7b64bb
Hope fix became clarere now.
There 2 problems in this post_meth_exit:
state->is_enabled(JVMTI_EVENT_METHOD_EXIT)however transition in the JRT_BLOCK_END happens always. So there is a risk of loosing method results in the interp_only mode.get_jvmti_thread_stateshould be called when thread is in vm state only.The fix adds
post_method_exit_transitionto have single exit point with oop restoration.Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27112/head:pull/27112$ git checkout pull/27112Update a local copy of the PR:
$ git checkout pull/27112$ git pull https://git.openjdk.org/jdk.git pull/27112/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27112View PR using the GUI difftool:
$ git pr show -t 27112Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27112.diff
Using Webrev
Link to Webrev Comment